@binste I know I removed ChannelType here, but altair.typing could define the precise types for each of the positional-args.

pola-rs/polars#17995 (comment)

E.g. ChannelX, ChannelY, ..., or similar?

These did vary between each method, but I think this would be 6 unique channel type aliases total:

• https://github.com/MarcoGorelli/polars/blob/db6d8f76782aa02bb72b923cf46e2f638ede6bef/py-polars/polars/dataframe/plotting.py#L30
• https://github.com/MarcoGorelli/polars/blob/db6d8f76782aa02bb72b923cf46e2f638ede6bef/py-polars/polars/dataframe/plotting.py#L92
• https://github.com/MarcoGorelli/polars/blob/db6d8f76782aa02bb72b923cf46e2f638ede6bef/py-polars/polars/dataframe/plotting.py#L161

Unsure if there would be more needed for the yet-unwritten methods

You already got very far! Great to see that we're almost there 😄

I'm going through it now but I'm also wondering if we maybe just want to specify, for each channel, the types separately. ChannelX, ChannelY, etc. works for me regarding naming. So basically what's there now for EncodingKwgs but as individual type aliases.

I don't know if we then even need a TypedDict, what do you think? Unpack supports the usage suggested in pola-rs/polars#17995 (comment) only for Python >= 3.12. And I think the plot methods should then only accept **kwargs: Unpack[EncodeKwgs] as else the arguments which are explicitly defined are defined twice. See typing docs.

Downside would be that in Polars, users only get autocomplete and type checking for the channels which are explicitly defined but for simple exploratory charts, this could be enough?

You already got very far! Great to see that we're almost there 😄

Thanks @binste for such a thorough response!

And I think the plot methods should then only accept **kwargs: Unpack[EncodeKwds] as else the arguments which are explicitly defined are defined twice. See typing docs.

I was trying to demonstrate the correct way to use Unpack there, to avoid that issue (the second one):

def foo(name, **kwargs: Unpack[Movie]) -> None: ...     # WRONG! "name" will always bind to the first parameter.

def foo(name, /, **kwargs: Unpack[Movie]) -> None: ...  # OK! "name" is a positional-only parameter,
           # ^^^                                        # so **kwargs can contain a "name" keyword.

I don't know if we then even need a TypedDict, what do you think? Unpack supports the usage suggested in pola-rs/polars#17995 (comment) only for Python >= 3.12.

I'm not sure there's an issue with typing_extensions.Unpack which I used unconditionally there?
Usually ruff or pylance will warn me when I do something that isn't allowed in py3.8

@binste

Downside would be that in Polars, users only get autocomplete and type checking for the channels which are explicitly defined but for simple exploratory charts, this could be enough?

@dangotbanned comment

You could then repeat the /, **kwargs: Unpack[EncodeKwds] for the other methods - maybe changing the positional-only ones if needed

This was essentially the compromise of that, if that makes sense?
You have your common ones that can be called without kwargs, but more options with kwargs

Yes, you're absolutely right! Thanks for the detailed explanation. Lost a bit the overview there but this brought me back on track :)

Just tested /, **kwargs: Unpack[EncodeKwds] with typing_extensions.Unpack on Python 3.11 and works as expected.

Reviewing the rest now.

@MarcoGorelli when you see this

The need for this solution is that alt.Chart.encode has alphabetically ordered parameters, whereas pl.DataFrame.plot has:

• pos-only (in a different order)
• *args
• **kwargs

Marking as unresolved purely so this is easier to find during review

It is actually resolved unless #3515 (comment) turns up anything new

@binste definitely agree with having a section for this in the API reference!

I did some testing though and my suggestion would be holding off on this until after 5.4.0.
We'll probably need to experiment a bit to get everything looking consistent, and I wouldn't want that blocking @MarcoGorelli getting their hands altair.typing for polars.

We could still keep the relevant code, but just leave it out of the toctree for now?

I've had my eye on switching to sphinx-autoapi to more easily customise when-then-otherwise. The jsonschema docs are built with it instead of sphinx-autodoc.

I think these could be handled together in another issue focused on the API reference.
What do you think?

As we now define our public API via the API section (#3488) I think we should offer this guarantee to polars that there are no breaking changes in altair.typing within minor versions. As 5.4.0 will likely be the minimum required version, it would be great to already have it.

I fully agree with you that the docs can be made more user-friendly for these objects. However, I do not (yet) see it as a blocking issue. When I generate the docs, all type hint aliases are correctly identified by sphinx as such, nothing is identified as TypeAliasType as in your screenshot:

Did you modify anything to produce this on purpose or do we have a difference? Could you maybe try again with hatch run doc:clean-generated && hatch run doc:build-html?

As we now define our public API via the API section (#3488) I think we should offer this guarantee to polars that there are no breaking changes in altair.typing within minor versions. As 5.4.0 will likely be the minimum required version, it would be great to already have it.

Yeah you do make a good point @binste, great to see (#3488) mentioned!

I fully agree with you that the docs can be made more user-friendly for these objects. However, I do not (yet) see it as a blocking issue. When I generate the docs, all type hint aliases are correctly identified by sphinx as such, nothing is identified as TypeAliasType as in your screenshot:

So I don't think any of them were identified with the labels (TypeAlias, TypeAliasType, ..., TypedDict) I gave in #3515 (comment).
I used those in the comment to explain why sphinx had treated them differently.

Did you modify anything to produce this on purpose or do we have a difference? Could you maybe try again with hatch run doc:clean-generated && hatch run doc:build-html?

Didn't modify anything to produce that intentionally.
I run on a clean build, here is another screenshot w/ another build - in case this is sphinx version related

Maybe the issue is hatch isn't updating your sphinx, since the dependencies don't state a minimum version?

Otherwise, maybe it is the python version we're using. I'm using 3.12, which would be the first one to introduce TypeAliasType.
Is it possible that sphinx would only use the symbol if built using sys.version_info >= (3, 12)?
That seems odd to me, as this was using typing_extensions.

@binste hopefully we'll get the same sphinx output now I've merged this #3527

Weird, even with #3527, I don't get this. I'm using Python 3.11 so maybe it's something like this...

I can build the docs with this setup when making the release. I'd suggest that if I merge this on the weekend and then make the release as-is. This makes it part of the public API and we can figure out doc issues at a later stage. Let me know if I'm missing anything, thanks! :)

Weird, even with #3527, I don't get this. I'm using Python 3.11 so maybe it's something like this...

I can build the docs with this setup when making the release. I'd suggest that if I merge this on the weekend and then make the release as-is. This makes it part of the public API and we can figure out doc issues at a later stage. Let me know if I'm missing anything, thanks! :)

I agree @binste, if you have consistency with 3.11 then go with that for now 👍

Also, and this is non-blocking for me, but I'd suggest renaming the section from Type Hints -> Typing

Has been fun working with you on this

Done 👍 And same, was fun :)